Skip to content

fix(memory): Fix memory leak for audio events when pausing the game#2731

Open
Caball009 wants to merge 1 commit into
TheSuperHackers:mainfrom
Caball009:fix_memory_leak_audio_requests
Open

fix(memory): Fix memory leak for audio events when pausing the game#2731
Caball009 wants to merge 1 commit into
TheSuperHackers:mainfrom
Caball009:fix_memory_leak_audio_requests

Conversation

@Caball009
Copy link
Copy Markdown

Pausing the game leaks audio events that were in the audio request container at the time. This PR fixes that.

This code is called to get rid of some of the audio requests when pausing the game:

//Get rid of PLAY audio requests when pausing audio.
std::list<AudioRequest*>::iterator ait;
for (ait = m_audioRequests.begin(); ait != m_audioRequests.end(); /* empty */)
{
AudioRequest *req = (*ait);
if( req && req->m_request == AR_Play )
{
deleteInstance(req);
ait = m_audioRequests.erase(ait);
}
else
{
ait++;
}
}

AudioRequest::m_pendingEvent can be an owning raw pointer, though, which the destructor of AudioRequest should delete when needed. It currently doesn't, which is why the leak happens.

There's one exception where the ownership of the audio event is taken away from the audio request:

@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Memory Is memory related Fix Is fixing something, but is not user facing labels May 19, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 19, 2026

Greptile Summary

This PR fixes a memory leak that occurred when pausing the game while audio play requests were queued. The leak happened because AudioRequest::~AudioRequest() did not delete m_pendingEvent, and any queued AR_Play requests discarded at pause time left their AudioEventRTS* dangling.

  • AudioRequest.cpp: Adds a destructor body that deletes m_pendingEvent when m_usePendingEvent is true, covering the pause-discard path and any other path that destroys a play request before processing it (e.g., checkForSample failures).
  • MilesAudioManager.h/.cpp: Changes playAudioEvent to accept AudioEventRTS*& (reference to pointer) so it can null out the caller's pointer immediately after transferring ownership to PlayingAudio::m_audioEventRTS, preventing the destructor from double-deleting an already-owned event on the normal play path.

Confidence Score: 5/5

The change is a well-scoped memory leak fix with no behavioural side effects on the normal play path.

Ownership transfer is handled correctly in all three audio branches (stream, 3D, 2D). The early-return path in playAudioEvent when getAudioEventInfo() returns null leaves the pointer non-null, but the destructor then correctly deletes it. The delete nullptr on the normal play path is defined no-op behaviour in C++. The m_usePendingEvent guard properly prevents the destructor from misinterpreting the union as a pointer for pause/stop requests.

No files require special attention.

Important Files Changed

Filename Overview
Core/GameEngine/Source/Common/Audio/AudioRequest.cpp Adds destructor body that correctly deletes m_pendingEvent when m_usePendingEvent is true; delete nullptr is safe in C++ for the normal-play path where ownership was already transferred.
Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h Signature change for playAudioEvent from raw pointer to reference-to-pointer to enable ownership nulling at the call site.
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp Implements the reference-to-pointer signature; all three audio branches (stream, 3D, 2D) null out the event pointer immediately after assigning to audio->m_audioEventRTS, and downstream uses are correctly switched to audio->m_audioEventRTS.

Sequence Diagram

sequenceDiagram
    participant PL as processRequestList
    participant PR as processRequest
    participant PA as playAudioEvent
    participant AR as ~AudioRequest

    Note over PL: Normal play path
    PL->>PR: processRequest(req)
    PR->>PA: "playAudioEvent(req->m_pendingEvent)"
    PA->>PA: "audio->m_audioEventRTS = event"
    PA->>PA: "event = nullptr (nulls req->m_pendingEvent)"
    PA-->>PR: return
    PR-->>PL: return
    PL->>AR: deleteInstance(req)
    AR->>AR: "m_usePendingEvent=true, m_pendingEvent=nullptr"
    AR->>AR: delete nullptr (no-op, safe)

    Note over PL: Pause / discard path
    PL->>AR: deleteInstance(req) [skips processRequest]
    AR->>AR: "m_usePendingEvent=true, m_pendingEvent!=nullptr"
    AR->>AR: delete m_pendingEvent (leak fixed!)
Loading

Reviews (3): Last reviewed commit: "fix(memory): Fix memory leak for audio e..." | Re-trigger Greptile

Comment thread Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp Outdated
Comment thread Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp Outdated
@Caball009 Caball009 force-pushed the fix_memory_leak_audio_requests branch from 057637e to 977bd8b Compare May 19, 2026 21:15
Copy link
Copy Markdown

@xezon xezon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good.

@Caball009 Caball009 force-pushed the fix_memory_leak_audio_requests branch from 977bd8b to afb337c Compare May 19, 2026 22:15
@Caball009
Copy link
Copy Markdown
Author

Rebased to include the fix for the CI Replay checker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fix Is fixing something, but is not user facing Gen Relates to Generals Memory Is memory related Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants